Remove the dotnetSdkInstallationEnabled feature flag#14811
Remove the dotnetSdkInstallationEnabled feature flag#14811davidfowl merged 1 commit intorelease/13.2from
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14811Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14811" |
There was a problem hiding this comment.
Pull request overview
This PR removes the CLI’s automatic .NET SDK installation capability (and its feature flag), simplifying the SDK “installer” into a pure availability check while keeping the CLI fail-fast behavior when the required SDK isn’t present.
Changes:
- Removed
DotNetSdkInstallationEnabledfeature flag (CLI + VS Code extension schemas) and deleted embedded dotnet-install scripts/workflow. - Simplified
IDotNetSdkInstaller/DotNetSdkInstallerto only check for SDK availability (no install path). - Updated call sites and tests to reflect the new check-only behavior and telemetry outcomes.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Utils/SdkInstallHelperTests.cs | Updates tests for check-only flow and revised telemetry tags. |
| tests/Aspire.Cli.Tests/TestServices/TestDotNetSdkInstaller.cs | Updates test stub to the new CheckAsync tuple signature. |
| tests/Aspire.Cli.Tests/Templating/DotNetTemplateFactoryTests.cs | Adjusts template tests for updated CheckAsync return shape. |
| tests/Aspire.Cli.Tests/DotNetSdkInstallerTests.cs | Removes install-script expectations and updates checks to new tuple signature. |
| tests/Aspire.Cli.Tests/Commands/SdkInstallerTests.cs | Updates command tests to use the new CheckAsync tuple signature. |
| src/Aspire.Cli/Utils/SdkInstallResult.cs | Prunes enum values to reflect check-only outcomes. |
| src/Aspire.Cli/Utils/SdkInstallHelper.cs | Removes install UX; now only checks and displays an error on failure. |
| src/Aspire.Cli/Utils/SdkCheckResult.cs | Prunes enum values to reflect check-only outcomes. |
| src/Aspire.Cli/Utils/EnvironmentChecker/DotNetSdkCheck.cs | Updates to the new CheckAsync tuple signature. |
| src/Aspire.Cli/Templating/DotNetTemplateFactory.cs | Calls the simplified EnsureSdkInstalledAsync signature. |
| src/Aspire.Cli/Resources/dotnet-install.sh | Deleted (no longer embedding installer scripts). |
| src/Aspire.Cli/Resources/dotnet-install.ps1 | Deleted (no longer embedding installer scripts). |
| src/Aspire.Cli/Projects/DotNetAppHostProject.cs | Updates SDK check calls prior to run/publish. |
| src/Aspire.Cli/NuGet/NuGetPackagePrefetcher.cs | Updates to the new CheckAsync tuple signature. |
| src/Aspire.Cli/KnownFeatures.cs | Removes the dotnetSdkInstallationEnabled feature definition/metadata. |
| src/Aspire.Cli/DotNet/IDotNetSdkInstaller.cs | Removes install API; CheckAsync now returns a 3-tuple. |
| src/Aspire.Cli/DotNet/DotNetSdkInstaller.cs | Deletes installation logic and simplifies constructor/dependencies. |
| src/Aspire.Cli/DotNet/DotNetCliRunner.cs | Removes DOTNET_ROLL_FORWARD=LatestMajor injection tied to auto-install. |
| src/Aspire.Cli/Commands/InitCommand.cs | Updates SDK check call site for new helper signature. |
| src/Aspire.Cli/Commands/ExecCommand.cs | Updates SDK check call site for new helper signature. |
| src/Aspire.Cli/Commands/AddCommand.cs | Updates SDK check call site for new helper signature. |
| src/Aspire.Cli/Aspire.Cli.csproj | Removes embedded resource entries for installer scripts. |
| extension/schemas/aspire-settings.schema.json | Removes dotnetSdkInstallationEnabled setting. |
| extension/schemas/aspire-global-settings.schema.json | Removes dotnetSdkInstallationEnabled setting. |
| .github/workflows/update-dotnet-install-scripts.yml | Deleted (no longer maintaining embedded scripts). |
Comments suppressed due to low confidence (2)
tests/Aspire.Cli.Tests/Utils/SdkInstallHelperTests.cs:92
- The test
EnsureSdkInstalledAsync_WhenSdkMissing_DisplaysErrorno longer asserts that an error was actually displayed; it only checks telemetry tags. This makes the test name/intent misleading and leaves the user-facing error path unverified.
Set interactionService.DisplayErrorCallback to capture the message and assert it contains the minimum required version and the detected version.
public async Task EnsureSdkInstalledAsync_WhenSdkMissing_DisplaysError()
{
using var fixture = new TelemetryFixture();
var sdkInstaller = new TestDotNetSdkInstaller
{
CheckAsyncCallback = _ => (false, "8.0.100", "9.0.302")
};
var interactionService = new TestConsoleInteractionService();
var result = await SdkInstallHelper.EnsureSdkInstalledAsync(
sdkInstaller,
interactionService,
fixture.Telemetry);
Assert.False(result);
Assert.NotNull(fixture.CapturedActivity);
var tags = fixture.CapturedActivity.Tags.ToDictionary(t => t.Key, t => t.Value);
Assert.Equal("8.0.100", tags[TelemetryConstants.Tags.SdkDetectedVersion]);
Assert.Equal("9.0.302", tags[TelemetryConstants.Tags.SdkMinimumRequiredVersion]);
Assert.Equal("not_installed", tags[TelemetryConstants.Tags.SdkCheckResult]);
Assert.Equal("not_installed", tags[TelemetryConstants.Tags.SdkInstallResult]);
}
src/Aspire.Cli/Utils/SdkInstallResult.cs:19
SdkInstallResultis now documented as an “availability check”, but the type name still implies an installation result. With installation removed, this mismatch is easy to misread when maintaining telemetry/tagging code.
Consider renaming the enum to reflect the new behavior (or at least clarify in the XML doc that the name is retained for historical/telemetry reasons).
/// <summary>
/// Result of an SDK availability check.
/// </summary>
internal enum SdkInstallResult
{
/// <summary>
/// A valid SDK was already installed.
/// </summary>
AlreadyInstalled,
/// <summary>
/// The SDK is missing or does not meet the minimum required version.
/// </summary>
NotInstalled
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #22538430952 |
|
This looks like an issue with JSON formatting: |
b2a84e0 to
ab6ba2a
Compare
… logic - Remove the dotnetSdkInstallationEnabled feature flag from KnownFeatures - Remove SDK auto-install prompting logic from SdkInstallHelper - Simplify EnsureSdkInstalledAsync to only check and report SDK availability - Remove unused fields from ExecCommand and InitCommand - Consolidate duplicate SdkCheckResult/SdkInstallResult enums into SdkCheckResult - Remove duplicate SdkInstallResult telemetry tag - Update tests accordingly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ab6ba2a to
2144011
Compare
What was the fix? |
|
I re-ran it, which means there's a hidden bug with how we're setting the output |
|
Filed this #14819 |
PR #14811 removed ForceInstall from IDotNetSdkInstaller.CheckAsync, changing from 4-element to 3-element tuple. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Users are responsible for acquiring .NET SDK. The CLI still fails fast when the required SDK version is absent, but no longer offers automatic download/installation.
Feature flag removal
DotNetSdkInstallationEnabledfromKnownFeatures.csand both VS Code extension schemasSDK installer simplification
IDotNetSdkInstaller: removedInstallAsync;CheckAsyncreturns(Success, HighestDetectedVersion, MinimumRequiredVersion)— droppedForceInstallDotNetSdkInstaller: removedInstallAsync, all install helpers,forceInstall/alwaysInstallSdklogic, private SDK dir check, unused constructor params (executionContext,dotNetCliRunner,logger)SdkInstallHelper.EnsureSdkInstalledAsync: removedfeaturesandhostEnvironmentparams; now just checks and displays error on failureSdkInstallResult/SdkCheckResult: pruned toAlreadyInstalled/NotInstalledRuntime behavior
DOTNET_ROLL_FORWARD=LatestMajorinjection fromDotNetCliRunner(only needed to support auto-installed SDKs)Deleted files
src/Aspire.Cli/Resources/dotnet-install.shsrc/Aspire.Cli/Resources/dotnet-install.ps1.github/workflows/update-dotnet-install-scripts.ymlChecklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue:Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
centralus-2.in.applicationinsights.azure.com/home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests --internal-msbuild-node /home/REDACTED/.local/share/fbbd10cf7c5c45b9a1742c769b8bd30c/.p --filter-not-trait category=failing --filter-not-trait quarantined=true --filter-not-trait outerloop=true(dns block)/usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.deps.json /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Microsoft.DotNet.RemoteExecutor.dll Aspire.Cli.Tests, Version=42.42.42.42, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 Aspire.Cli.Tests.CliSmokeTests+<>c <LocaleOverrideReturnsExitCode>b__4_0 /tmp/bfvrwdhr.gc1 invalid-locale False ASPIRE_LOCALE_OVERRIDE(dns block)/usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.deps.json /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Microsoft.DotNet.RemoteExecutor.dll Aspire.Cli.Tests, Version=42.42.42.42, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 Aspire.Cli.Tests.CliSmokeTests+<>c <LocaleOverrideReturnsExitCode>b__4_0 /tmp/bfvrwdhr.gc1(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
This section details on the original issue you should resolve
<issue_title>Remove the dotnetSdkInstallationEnabled feature flag</issue_title>
<issue_description>We've decided that the user is responsible to acquiring .NET for C# based apphosts.</issue_description>
<agent_instructions>Users are responsible for acquiring .NET for C# apphosts. The CLI still fails fast when the required SDK version is absent, but no longer offers automatic download and installation.
Changes
Feature flag removal
DotNetSdkInstallationEnabledfromKnownFeatures.csand both VS Code extension schemasSDK installer simplification
IDotNetSdkInstaller: removedInstallAsync;CheckAsyncnow returns(Success, HighestDetectedVersion, MinimumRequiredVersion)(droppedForceInstall)DotNetSdkInstaller: removedInstallAsync, all install helpers,forceInstall/alwaysInstallSdklogic, private SDK dir check, and unused constructor params (executionContext,dotNetCliRunner,logger)SdkInstallHelper.EnsureSdkInstalledAsync: removedfeaturesandhostEnvironmentparams; now just checks availability and displays an error on failureSdkInstallResult/SdkCheckResult: pruned toAlreadyInstalled/NotInstalledonlyRuntime behavior
DOTNET_ROLL_FORWARD=LatestMajorinjection fromDotNetCliRunner(was needed only to support auto-installed SDKs)Deleted files
src/Aspire.Cli/Resources/dotnet-install.shsrc/Aspire.Cli/Resources/dotnet-install.ps1.github/workflows/update-dotnet-install-scripts.ymlTests: updated
TestDotNetSdkInstaller,SdkInstallHelperTests,DotNetSdkInstallerTests, andSdkInstallerTeststo reflect the simplified interface.Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue:💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.